Skip to content

feat: Add types for canister snapshot visibility settings#9158

Merged
gregorydemay merged 21 commits intomasterfrom
gdemay/DEFI-2667-snapshot-visibility-settings
Mar 11, 2026
Merged

feat: Add types for canister snapshot visibility settings#9158
gregorydemay merged 21 commits intomasterfrom
gdemay/DEFI-2667-snapshot-visibility-settings

Conversation

@gregorydemay
Copy link
Copy Markdown
Contributor

@gregorydemay gregorydemay commented Mar 3, 2026

Add the types needed for canisters' snapshot visibility settings, to allow principals other than the canister's controllers to be able to list and read canister's snaphots. In short this is like log visibility settings but for canister's snapshots. This PR only adds the needed types and does not change any existing logic.

Support for snapshot visibility settings for NNS-owned canisters will be done in #9257. Note that the changes regarding the CMC are needed in this PR because the CMC exposes ic_management_canister_types_private::CanisterSettingsArgs via the type CreateCanister which is an argument of the create_canister endpoint and the test test_candid_interface_compatibility ensures that the exposed Canister API is equal to the one declared in cmc.did.

Sequence of PRs to implement canister snapshot visibility settings as specified in dfinity/portal#6195:

  1. refactor(exec): General-purpose visibility settings #9155
  2. feat: Add types for canister snapshot visibility settings #9158 (this)
  3. feat(exec): List and read canister snapshots according to new visibility settings #9227

@github-actions github-actions Bot added the feat label Mar 3, 2026
@gregorydemay gregorydemay added the CI_ALL_BAZEL_TARGETS Runs all bazel targets label Mar 4, 2026
@gregorydemay gregorydemay changed the title feat: Canister snapshot visibility settings feat: Add types for canister snapshot visibility settings Mar 6, 2026
Comment thread rs/nns/nns.bzl Outdated
Comment thread rs/nervous_system/clients/src/update_settings.rs Outdated
Comment thread rs/nns/cmc/cmc.did Outdated
gregorydemay and others added 2 commits March 9, 2026 11:15
These changes are moved to a separate PR on branch
gdemay/DEFI-2667-snapshot-visibility-governance.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
github-merge-queue Bot pushed a commit that referenced this pull request Mar 9, 2026
Extract logic to authorize access to some canister data (e.g., logs,
snapshots) based on some visibility settings.

Sequence of PRs to implement canister snapshot visibility settings as
specified in dfinity/portal#6195:
1. #9155 (this)
2. #9158
3.  #9227

---------

Co-authored-by: IDX GitHub Automation <[email protected]>
Base automatically changed from gdemay/DEFI-2667-visibility-settings to master March 9, 2026 16:07
# Conflicts:
#	rs/cycles_account_manager/src/lib.rs
#	rs/execution_environment/src/canister_settings.rs
@gregorydemay gregorydemay marked this pull request as ready for review March 9, 2026 16:47
@gregorydemay gregorydemay requested a review from a team as a code owner March 9, 2026 16:47
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request changes code owned by the Governance team. Therefore, make sure that
you have considered the following (for Governance-owned code):

  1. Update unreleased_changelog.md (if there are behavior changes, even if they are
    non-breaking).

  2. Are there BREAKING changes?

  3. Is a data migration needed?

  4. Security review?

How to Satisfy This Automatic Review

  1. Go to the bottom of the pull request page.

  2. Look for where it says this bot is requesting changes.

  3. Click the three dots to the right.

  4. Select "Dismiss review".

  5. In the text entry box, respond to each of the numbered items in the previous
    section, declare one of the following:

  • Done.

  • $REASON_WHY_NO_NEED. E.g. for unreleased_changelog.md, "No
    canister behavior changes.", or for item 2, "Existing APIs
    behave as before.".

Brief Guide to "Externally Visible" Changes

"Externally visible behavior change" is very often due to some NEW canister API.

Changes to EXISTING APIs are more likely to be "breaking".

If these changes are breaking, make sure that clients know how to migrate, how to
maintain their continuity of operations.

If your changes are behind a feature flag, then, do NOT add entrie(s) to
unreleased_changelog.md in this PR! But rather, add entrie(s) later, in the PR
that enables these changes in production.

Reference(s)

For a more comprehensive checklist, see here.

GOVERNANCE_CHECKLIST_REMINDER_DEDUP

@gregorydemay gregorydemay dismissed github-actions[bot]’s stale review March 9, 2026 16:58

Updated CMC unreleased_changelog.md.

Copy link
Copy Markdown
Contributor

@mraszyk mraszyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DSM changes look good to me. The NNS changes are likely accidental here and should be moved to the separate PR.

@gregorydemay
Copy link
Copy Markdown
Contributor Author

DSM changes look good to me. The NNS changes are likely accidental here and should be moved to the separate PR.

@mraszyk The changes regarding the CMC are not accidental: they must be part of that PR because the CMC exposes ic_management_canister_types_private::CanisterSettingsArgs via the type CreateCanister which is an argument of the create_canister endpoint and the test test_candid_interface_compatibility ensures that the exposed Canister API is equal to the one declared in cmc.did. Not having those changes in this PR would require a bigger refactoring in the CMC code (basically to not expose the private ic_management_canister_types_private::CanisterSettingsArgs). I will adapt the PR description.

Comment thread rs/types/management_canister_types/src/lib.rs Outdated
shilingwang pushed a commit that referenced this pull request Mar 10, 2026
Extract logic to authorize access to some canister data (e.g., logs,
snapshots) based on some visibility settings.

Sequence of PRs to implement canister snapshot visibility settings as
specified in dfinity/portal#6195:
1. #9155 (this)
2. #9158
3.  #9227

---------

Co-authored-by: IDX GitHub Automation <[email protected]>
@gregorydemay gregorydemay enabled auto-merge March 11, 2026 16:49
@gregorydemay gregorydemay added this pull request to the merge queue Mar 11, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Mar 11, 2026
@gregorydemay gregorydemay added this pull request to the merge queue Mar 11, 2026
Merged via the queue into master with commit e227b3b Mar 11, 2026
39 checks passed
@gregorydemay gregorydemay deleted the gdemay/DEFI-2667-snapshot-visibility-settings branch March 11, 2026 19:39
github-merge-queue Bot pushed a commit that referenced this pull request Mar 16, 2026
…ity settings (#9227)

Allow access to the endpoints

- `read_canister_snapshot_metadata`
- `read_canister_snapshot_data`
- `list_canister_snapshots`

according the canister snapshot visibility settings.

Sequence of PRs to implement canister snapshot visibility settings as
specified in dfinity/portal#6195:
1. #9155 
2. #9158 
3. #9227 (this)

---------

Co-authored-by: mraszyk <[email protected]>
Co-authored-by: Martin Raszyk <[email protected]>
Co-authored-by: IDX GitHub Automation <[email protected]>
github-merge-queue Bot pushed a commit that referenced this pull request Mar 25, 2026
## Summary
- Follow-up on #9158 to add `snapshot_visibility` support to
governance-team owned canisters (NNS, SNS, CMC, registry admin)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
alin-at-dfinity pushed a commit that referenced this pull request Mar 26, 2026
## Summary
- Follow-up on #9158 to add `snapshot_visibility` support to
governance-team owned canisters (NNS, SNS, CMC, registry admin)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants